Show deprecation status of operators and sensors on the docs operator list page #1568
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pankajkoti
requested review from
phanikumv,
pankajastro,
Lee-W,
sunank200,
utkarsharma2 and
vatsrahul1001
as code owners
|
Built docs preview
|
pankajkoti
force-pushed
the
show-deprecation-status-on-operartor-list-doc
branch
from
65bd735 to
9f4441f
Compare
pankajkoti
force-pushed
the
show-deprecation-status-on-operartor-list-doc
branch
2 times, most recently
from
0fb130f to
b24499b
Compare
pankajkoti
force-pushed
the
show-deprecation-status-on-operartor-list-doc
branch
from
b24499b to
d355144
Compare
kaxil
reviewed
Aug 19, 2024
| sphinx | ||
| sphinx-autoapi | ||
| sphinx-copybutton | ||
| astronomer-providers[all] |
There was a problem hiding this comment.
Hmm, this is recursive, no?
You just need to install:
pip install -e .[docs]
There was a problem hiding this comment.
no, it does not cause a recursion as all does not include the dependencies for docs, but the docs now need all the packages to be installed to evaluate the attributes on the classes.
I mean the build-docs job did not complain for a recursion but without that it complains for each of the missing providers that we're using to get the attributes for.
kaxil
approved these changes
Aug 19, 2024
There was a problem hiding this comment.
Nice, alternative way if you didn't wanted to change the source code:
diff --git a/docs/_ext/traverse_operators_sensors.py b/docs/_ext/traverse_operators_sensors.py
index 13891b1..9078036 100644
--- a/docs/_ext/traverse_operators_sensors.py
+++ b/docs/_ext/traverse_operators_sensors.py
@@ -2,6 +2,7 @@ import ast
import os
from pathlib import Path
from typing import List, Tuple
+import re
from docutils import nodes
from sphinx.util.docutils import SphinxDirective
@@ -22,12 +23,13 @@ def collect_elements(
directory_path: str,
files: List[str],
element_type: str,
- elements_list: List[Tuple[str, str]],
+ elements_list: List[Tuple[str, str, bool]],
current_module_path: str,
):
"""
Checks that ``Async`` class definitions exist using ``ast.parse`` in the given files, that those are
- ``operators/sensors`` and appends the operator/sensor along with its import path to the given output list.
+ ``operators/sensors`` and appends the operator/sensor along with its import path and deprecation status
+ to the given output list.
:param directory_path: path of the directory in which the given ``files`` are located
:param files: list of files to look for async operator/sensor class definitions
@@ -47,7 +49,35 @@ def collect_elements(
if isinstance(element, ast.ClassDef):
element_name = element.name
if element_type in element_name and ASYNC_SUBSTRING in element_name:
- elements_list.append((element_name, module_import_path))
+ is_deprecated = False
+ replacement_class = ""
+ for class_element in element.body:
+ if isinstance(class_element, ast.FunctionDef) and class_element.name == "__init__":
+ for stmt in class_element.body:
+ if isinstance(stmt, ast.Expr) and isinstance(stmt.value, ast.Call):
+ if isinstance(stmt.value.func, ast.Attribute):
+ if stmt.value.func.attr == "warn" and stmt.value.func.value.id == "warnings":
+ is_deprecated = True
+ # Concatenate the warning message
+ warning_message = ""
+ for arg in stmt.value.args:
+ if isinstance(arg, ast.Str):
+ warning_message += arg.s
+ # Extract the replacement class from the concatenated message
+ match = re.search(
+ r":class: `~([^`]+)`|`([^`]+)`",
+ warning_message
+ )
+ if match:
+ replacement_class_path = match.group(1) or match.group(2)
+ # Transform to import statement format
+ if replacement_class_path:
+ parts = replacement_class_path.rsplit('.', 1)
+ if len(parts) == 2:
+ replacement_class = (
+ f"from {parts[0]} import {parts[1]}"
+ )
+ elements_list.append((element_name, is_deprecated, module_import_path, replacement_class))
def search_providers():
@@ -79,15 +109,18 @@ class TraverseOperatorsSensors(SphinxDirective):
"""Generates raw html to list the operators and sensors parsed using `ast`."""
operators, sensors = search_providers()
operators_html = (
- "<h3>Operators</h3> " "<table>" "<th>#</th>" "<th>Operator name</th>" "<th>Import path</th>"
+ "<h3>Operators</h3> " "<table>" "<th>#</th>" "<th>Operator name</th>" "<th>Deprecated </th>"
+ "<th>Import path</th>" "<th>Replacement import path </th>"
)
for index, operator in enumerate(operators, start=1):
- class_def_link = operator[1].replace(".", "/") + "/index.html#" + operator[1] + "." + operator[0]
+ class_def_link = operator[2].replace(".", "/") + "/index.html#" + operator[2] + "." + operator[0]
operators_html += (
f"<tr>"
f"<td>{index}</td>"
f"<td><span><a id={operator[0]}>{operator[0]}</a></span></td>"
- f"<td><span><pre><code class='python'>from {operator[1]} import {operator[0]}</code></pre></span></td>"
+ f"<td><span>{'✅' if operator[1] else '❌' }</span></td>"
+ f"<td><span><pre><code class='python'>from {operator[2]} import {operator[0]}</code></pre></span></td>"
+ f"<td><span><pre><code class='python'>{operator[3]}</code></pre></span></td>"
f"</tr>"
)
# The below script generates the URL for the class definition by extracting the selected doc version from
@@ -101,14 +134,19 @@ class TraverseOperatorsSensors(SphinxDirective):
)
operators_html += "</table> <br/>"
- sensors_html = "<h3>Sensors</h3>" "<table>" "<th>#</th>" "<th>Sensor name</th>" "<th>Import path</th>"
+ sensors_html = (
+ "<h3>Sensors</h3>" "<table>" "<th>#</th>" "<th>Sensor name</th>" "<th>Deprecated </th>"
+ "<th>Import path</th>" "<th>Replacement import path </th>"
+ )
for index, sensor in enumerate(sensors, start=1):
- class_def_link = sensor[1].replace(".", "/") + "/index.html#" + sensor[1] + "." + sensor[0]
+ class_def_link = sensor[2].replace(".", "/") + "/index.html#" + sensor[2] + "." + sensor[0]
sensors_html += (
f"<tr>"
f"<td>{index}</td>"
f"<td><span><a id={sensor[0]}>{sensor[0]}</a></span></td>"
- f"<td><span><pre><code class='python'>from {sensor[1]} import {sensor[0]}</code></pre></span></td>"
+ f"<td><span>{'✅' if sensor[1] else '❌' }</span></td>"
+ f"<td><span><pre><code class='python'>from {sensor[2]} import {sensor[0]}</code></pre></span></td>"
+ f"<td><span><pre><code class='python'>{sensor[3]}</code></pre></span></td>"
f"</tr>"
)
sensors_html += (
|
Alternatively, you can have a single column with old & new path:
Git diff for that: diff --git a/docs/_ext/traverse_operators_sensors.py b/docs/_ext/traverse_operators_sensors.py
index 13891b1..4304a29 100644
--- a/docs/_ext/traverse_operators_sensors.py
+++ b/docs/_ext/traverse_operators_sensors.py
@@ -2,6 +2,7 @@ import ast
import os
from pathlib import Path
from typing import List, Tuple
+import re
from docutils import nodes
from sphinx.util.docutils import SphinxDirective
@@ -22,12 +23,13 @@ def collect_elements(
directory_path: str,
files: List[str],
element_type: str,
- elements_list: List[Tuple[str, str]],
+ elements_list: List[Tuple[str, str, bool]],
current_module_path: str,
):
"""
Checks that ``Async`` class definitions exist using ``ast.parse`` in the given files, that those are
- ``operators/sensors`` and appends the operator/sensor along with its import path to the given output list.
+ ``operators/sensors`` and appends the operator/sensor along with its import path and deprecation status
+ to the given output list.
:param directory_path: path of the directory in which the given ``files`` are located
:param files: list of files to look for async operator/sensor class definitions
@@ -47,7 +49,35 @@ def collect_elements(
if isinstance(element, ast.ClassDef):
element_name = element.name
if element_type in element_name and ASYNC_SUBSTRING in element_name:
- elements_list.append((element_name, module_import_path))
+ is_deprecated = False
+ replacement_class = ""
+ for class_element in element.body:
+ if isinstance(class_element, ast.FunctionDef) and class_element.name == "__init__":
+ for stmt in class_element.body:
+ if isinstance(stmt, ast.Expr) and isinstance(stmt.value, ast.Call):
+ if isinstance(stmt.value.func, ast.Attribute):
+ if stmt.value.func.attr == "warn" and stmt.value.func.value.id == "warnings":
+ is_deprecated = True
+ # Concatenate the warning message
+ warning_message = ""
+ for arg in stmt.value.args:
+ if isinstance(arg, ast.Str):
+ warning_message += arg.s
+ # Extract the replacement class from the concatenated message
+ match = re.search(
+ r":class: `~([^`]+)`|`([^`]+)`",
+ warning_message
+ )
+ if match:
+ replacement_class_path = match.group(1) or match.group(2)
+ # Transform to import statement format
+ if replacement_class_path:
+ parts = replacement_class_path.rsplit('.', 1)
+ if len(parts) == 2:
+ replacement_class = (
+ f"from {parts[0]} import {parts[1]}"
+ )
+ elements_list.append((element_name, is_deprecated, module_import_path, replacement_class))
def search_providers():
@@ -79,17 +109,29 @@ class TraverseOperatorsSensors(SphinxDirective):
"""Generates raw html to list the operators and sensors parsed using `ast`."""
operators, sensors = search_providers()
operators_html = (
- "<h3>Operators</h3> " "<table>" "<th>#</th>" "<th>Operator name</th>" "<th>Import path</th>"
+ "<h3>Operators</h3> " "<table>" "<th>#</th>" "<th>Operator name</th>" "<th>Deprecated </th>"
+ "<th>Import path</th>"
)
for index, operator in enumerate(operators, start=1):
- class_def_link = operator[1].replace(".", "/") + "/index.html#" + operator[1] + "." + operator[0]
- operators_html += (
- f"<tr>"
- f"<td>{index}</td>"
- f"<td><span><a id={operator[0]}>{operator[0]}</a></span></td>"
- f"<td><span><pre><code class='python'>from {operator[1]} import {operator[0]}</code></pre></span></td>"
- f"</tr>"
- )
+ class_def_link = operator[2].replace(".", "/") + "/index.html#" + operator[2] + "." + operator[0]
+ if operator[1]:
+ operators_html += (
+ f"<tr>"
+ f"<td>{index}</td>"
+ f"<td><span><a id={operator[0]}>{operator[0]}</a></span></td>"
+ f"<td style='text-align: center;'>✅</td>"
+ f"<td><b>Old Path:</b>\n <span><pre><code class='python'>from {operator[2]} import {operator[0]}</code></pre></span>\n <b>New Path:</b> \n<span><pre><code class='python'>{operator[3]}</code></pre></span></td>"
+ f"</tr>"
+ )
+ else:
+ operators_html += (
+ f"<tr>"
+ f"<td>{index}</td>"
+ f"<td><span><a id={operator[0]}>{operator[0]}</a></span></td>"
+ f"<td style='text-align: center;'>❌</td>"
+ f"<td><span><pre><code class='python'>from {operator[2]} import {operator[0]}</code></pre></span></td>"
+ f"</tr>"
+ )
# The below script generates the URL for the class definition by extracting the selected doc version from
# the current browser URL location.
operators_html += (
@@ -101,16 +143,30 @@ class TraverseOperatorsSensors(SphinxDirective):
)
operators_html += "</table> <br/>"
- sensors_html = "<h3>Sensors</h3>" "<table>" "<th>#</th>" "<th>Sensor name</th>" "<th>Import path</th>"
+ sensors_html = (
+ "<h3>Sensors</h3>" "<table>" "<th>#</th>" "<th>Sensor name</th>" "<th>Deprecated </th>"
+ "<th>Import path</th>"
+ )
for index, sensor in enumerate(sensors, start=1):
- class_def_link = sensor[1].replace(".", "/") + "/index.html#" + sensor[1] + "." + sensor[0]
- sensors_html += (
- f"<tr>"
- f"<td>{index}</td>"
- f"<td><span><a id={sensor[0]}>{sensor[0]}</a></span></td>"
- f"<td><span><pre><code class='python'>from {sensor[1]} import {sensor[0]}</code></pre></span></td>"
- f"</tr>"
- )
+ class_def_link = sensor[2].replace(".", "/") + "/index.html#" + sensor[2] + "." + sensor[0]
+ if sensor[1]:
+ sensors_html += (
+ f"<tr>"
+ f"<td>{index}</td>"
+ f"<td><span><a id={sensor[0]}>{sensor[0]}</a></span></td>"
+ f"<td style='text-align: center;'>✅</td>"
+ f"<td><b>Old Path:</b>\n <span><pre><code class='python'>from {sensor[2]} import {sensor[0]}</code></pre></span>\n <b>New Path:</b> \n<span><pre><code class='python'>{sensor[3]}</code></pre></span></td>"
+ f"</tr>"
+ )
+ else:
+ sensors_html += (
+ f"<tr>"
+ f"<td>{index}</td>"
+ f"<td><span><a id={sensor[0]}>{sensor[0]}</a></span></td>"
+ f"<td style='text-align: center;'>❌</td>"
+ f"<td><span><pre><code class='python'>from {sensor[2]} import {sensor[0]}</code></pre></span></td>"
+ f"</tr>"
+ )
sensors_html += (
f"<script> "
f"var base_url = '' + window.location.pathname.split('/providers/')[0] + '/_api/';" |
|
The change you have is also sufficient and does not rely on regex which is good :) so don't feel obligated to change anything. I just wanted to show more options |
Thanks Kaxil for the efforts in showing the alternatives. Mine is definitely some fast-food but yours is food cooked with love :) For now, I have applied the suggestion to improve the visualisation by having a single column for the import path with the old-new path approach and also including the visual ✅ & ❌ for deprecation status. I am also equally inclined to prefer the definite attribute approach compared to the regex searching for warning texts, so kept that logic as it is, hope that's okay 🤞🏽 Docs preview with the latest commit after applying the suggestion |
for more information, see https://pre-commit.ci
pankajkoti
force-pushed
the
show-deprecation-status-on-operartor-list-doc
branch
from
e1c4590 to
e1e602a
Compare
|
I'm merging this PR but please leave a comment if there are more suggestions and I will address them on priority, thanks for the review and suggestions! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces enhancements to the documentation for operators and sensors, specifically to indicate their deprecation status and suggest replacements where applicable.
Changes
New Attributes
Added
is_deprecatedandpost_deprecation_replacementattributes to operator and sensor classes.is_deprecatedattribute is a boolean that flags whether a class is deprecated.post_deprecation_replacementattribute provides a string with the import path for the recommended replacement.Script changes that builds the operators list
The script
docs/_ext/traverse_operators_sensors.pythat builds in to the documentation the list of operators and sensors now uses the above newly added attributes to denote whether the operators have been deprecatedcloses: #1567